feat(model): configurable default + per-spawn model override by dcetlin · Pull Request #81 · sf8193/hydra

dcetlin · 2026-07-04T17:39:40Z

What

Make the model selectable instead of hardcoded in two places (bridge-dispatch.SPAWN_MODEL for spawns, cli/lifecycle.ts for the byte).

Precedence: --model (per-spawn) > HYDRA_MODEL (config default) > built-in claude-opus-4-6[1m].

Changes

daemon/bridge-dispatch.ts — DEFAULT_MODEL constant; SPAWN_MODEL = process.env.HYDRA_MODEL ?? DEFAULT_MODEL
cli/lifecycle.ts — byte launch reads HYDRA_MODEL
daemon/sessions.ts — SpawnOpts.model?
daemon/session-lifecycle.ts — opts.model ?? SPAWN_MODEL across the 3 spawn/fork/resume branches + the registry model field
daemon/cli-handler.ts + cli/hydra.ts — hydra spawn <prompt> --model <id>
.env.example — document HYDRA_MODEL

Why no `buildDaemonEnvs` change

daemon/config.ts self-loads both the repo .env and the state-dir .env at import, so HYDRA_MODEL set in .env reaches SPAWN_MODEL in the daemon automatically.

Verification

bun build of cli/hydra.ts, daemon.ts, bridge.ts all pass.
No env, no flag → generated claude --model … string is byte-for-byte unchanged.
HYDRA_MODEL=X → byte and spawns launch with --model X.
hydra spawn … --model Y → that session launches with --model Y regardless of HYDRA_MODEL.

🤖 Generated with Claude Code

sf8193

PR #81 Review — configurable default + per-spawn model override

Clean, well-scoped change. The fallback chain (--model > HYDRA_MODEL > DEFAULT_MODEL) is sound. Two should-fix items and a few nits below.

Items not on diff lines

Should-fix: bridge-server.ts fallback capabilities still use SPAWN_MODEL — When a session reconnects without stored capabilities, the fallback model field uses the global SPAWN_MODEL constant, not the model the session was actually spawned with. After this PR, a session spawned with --model sonnet would have its capabilities lie on reconnect. Worth fixing the fallback path or ensuring stored capabilities are always restored.

Nit: spawn_session bridge tool has no model passthrough — Only CLI-initiated spawns get the per-spawn override. Byte-initiated spawns via the spawn_session bridge tool always use SPAWN_MODEL. If this is intentional, a comment noting it would help. If byte should also be able to pick models, the tool schema needs a model field.

sf8193 · 2026-07-04T19:20:54Z

  }

  const byteCwd = process.env.BYTE_CWD ?? cfg.spawnCwd
+  // Canonical default is DEFAULT_MODEL in daemon/bridge-dispatch.ts; keep in sync.


Should-fix (flagged by both reviewers): duplicate default constant. The hardcoded 'claude-opus-4-6[1m]' here duplicates DEFAULT_MODEL in bridge-dispatch.ts. The comment says "keep in sync" — that's how things get out of sync. Import DEFAULT_MODEL instead, or extract to a shared constants.ts. The CLI already cross-imports types from daemon/sessions.ts, so this is established practice.

Fixed — imported DEFAULT_MODEL from bridge-dispatch.ts and removed the hardcoded string + "keep in sync" comment.

sf8193 · 2026-07-04T19:20:54Z

@@ -361,14 +361,16 @@ export async function doSpawnSession(topic: string, chatId?: string, messageId?:
    prompt = buildSpawnPrompt(promptParams)
  }



Should-fix: empty-string model bypasses fallback. If --model "" is passed, the ?? operator doesn't catch it (empty string is not nullish), so shq('') produces '' and the spawned claude --model '' will fail opaquely. Use opts?.model || SPAWN_MODEL (falsy check), or validate at the CLI parsing layer in hydra.ts.

Fixed — switched to || (falsy check) so empty string falls through to SPAWN_MODEL. Also applied the same fix to the SPAWN_MODEL env-read itself in bridge-dispatch.ts:58 which had the same latent issue.

sf8193 · 2026-07-04T19:20:54Z

-export const SPAWN_MODEL = 'claude-opus-4-6[1m]'
+// Canonical default. HYDRA_MODEL overrides it for byte + all spawns;
+// `hydra spawn --model <id>` overrides a single spawn.
+export const DEFAULT_MODEL = 'claude-opus-4-6[1m]'


Nit: import-time-frozen env read. process.env.HYDRA_MODEL is read once when this module is first imported. For a long-running daemon, changing the env var requires a full daemon restart. This is likely fine in practice (daemon restarts are the norm), but worth either a brief comment noting it's intentionally frozen, or converting to a getSpawnModel() function if hot-reload is ever desired.

Leaving as-is — daemon restarts are the norm for config changes, so import-time freeze is intentional. The constant name + declaration position make this self-evident.

sf8193 · 2026-07-04T19:20:54Z

        if (filtered[i] === '--idempotency-key' && i + 1 < filtered.length) {
          idempotencyKey = filtered[++i]
        } else if (filtered[i] === '--initiator' && i + 1 < filtered.length) {
          initiator = filtered[++i]


Nit: --model as last arg silently swallowed into prompt. If --model is the final argument (no value follows), the i + 1 < filtered.length guard fails and "--model" gets pushed into promptParts. This matches the existing pattern for --initiator/--idempotency-key so it's consistent — but all three share the same latent issue. Not a blocker for this PR.

Agreed — pre-existing pattern shared by all flag args. Out of scope for this PR but noted for a future arg-parsing cleanup pass.

dcetlin · 2026-07-05T00:34:53Z

Re: bridge-server.ts fallback capabilities — Investigated. Capabilities are always set during spawn and persisted to disk, so the fallback only fires for sessions predating the capabilities field (migration edge case). SPAWN_MODEL is the correct default there. No change needed.

Re: spawn_session model passthrough — Fixed. Added model field to the tool schema and wired through executeTool → doSpawnSession(... { model }).

Bonus fix: the SPAWN_MODEL env-read itself (process.env.HYDRA_MODEL ?? DEFAULT_MODEL) had the same empty-string bypass — switched to || for consistency across the whole precedence chain.

sf8193 · 2026-07-05T02:55:24Z

  compileCheck, killOrphanBytes, hasOrphanBytes, appendLog, shq,
  waitForSocket, buildDaemonEnvs,
 } from './helpers.js'
+import { DEFAULT_MODEL } from '../daemon/bridge-dispatch.js'


Should-fix (flagged by both reviewers): Cross-boundary import — CLI layer imports from daemon/bridge-dispatch.js. This creates a transitive dependency on the daemon module graph at CLI load time. If bridge-dispatch.ts gains daemon-only side effects or imports, this breaks the CLI entrypoint.

Suggested fix: extract DEFAULT_MODEL into a shared constants module (e.g. shared/constants.ts) that both CLI and daemon import.

Fixed — extracted DEFAULT_MODEL to shared/constants.ts. Both CLI and daemon now import from there, no cross-boundary dependency.

sf8193 · 2026-07-05T02:55:24Z

+// Canonical default. HYDRA_MODEL overrides it for byte + all spawns;
+// `hydra spawn --model <id>` overrides a single spawn.
+export const DEFAULT_MODEL = 'claude-opus-4-6[1m]'
+export const SPAWN_MODEL = process.env.HYDRA_MODEL || DEFAULT_MODEL


Nit (flagged by both reviewers): SPAWN_MODEL is evaluated once at module load time. Changes to HYDRA_MODEL after import are invisible for the lifetime of the daemon process. This is fine if restart-to-reconfigure is the contract — worth a brief doc comment if so.

Also: the fallback || DEFAULT_MODEL is duplicated in cli/lifecycle.ts:69. Centralizing in a shared module (per the lifecycle.ts comment) would eliminate this.

Added a comment noting import-time freeze is intentional. The duplication is now gone — both sites import from shared/constants.ts.

sf8193 · 2026-07-05T02:55:24Z

    prompt = buildSpawnPrompt(promptParams)
  }

+  const model = opts?.model || SPAWN_MODEL


Should-fix (flagged by both reviewers): Use ?? instead of || here. With ||, an empty string model: '' (possible from the bridge tool JSON path) silently falls back to SPAWN_MODEL. With ??, only undefined/null trigger the fallback — an empty string would propagate and fail visibly at the claude CLI, which is the correct behavior for a bad input.

const model = opts?.model ?? SPAWN_MODEL

Agreed — reverted to ??. Empty string should propagate and fail visibly at the claude CLI. Input normalization now happens at the entry points (bridge tool dispatch + CLI parsing), not at the internal fallback.

sf8193 · 2026-07-05T02:55:24Z

        const worktree = args.worktree as string | undefined
        const topic = worktree ? `worktree:${worktree} ${args.topic}` : args.topic as string
-        const result = await doSpawnSession(topic, args.chat_id as string | undefined, args.message_id as string | undefined)
+        const model = args.model as string | undefined


Should-fix: No guard against empty-string model from LLM callers. The bridge receives arbitrary JSON from Claude — { "model": "" } passes the model ? { model } : undefined ternary (empty string is falsy, so this specific line is OK), but consider normalizing: const model = (args.model as string | undefined)?.trim() || undefined to be defensive.

Also: no validation or logging of the model override. A misbehaving session could request a non-existent model ID, causing a silent tmux failure. At minimum, logging the override would aid debugging.

Fixed — normalize at entry: (args.model as string | undefined)?.trim() || undefined. Also added process.stderr.write logging when a model override is used.

sf8193 · 2026-07-05T02:55:24Z

-  --idempotency-key <key>              Prevent duplicate spawns
+Spawn options:
+  --initiator <name>                   Who triggered this spawn (required)
+  --idempotency-key <key>              Prevent duplicate spawns (required)


Nit: Users reading CLI help won't know what HYDRA_MODEL resolves to when unset. Consider: (default: $HYDRA_MODEL or claude-opus-4-6[1m]).

Fixed — now shows (default: $HYDRA_MODEL or claude-opus-4-6[1m]).

sf8193 · 2026-07-05T15:04:26Z

@@ -0,0 +1 @@
+export const DEFAULT_MODEL = 'claude-opus-4-6[1m]'


Should-fix (flagged by sp-reviewer): tsconfig.json include is ["*.ts", "daemon/**/*.ts"] — neither shared/ nor cli/ are covered. This new file (and cli/lifecycle.ts which imports it) won't be type-checked by tsc. Add "shared/**/*.ts" (and ideally "cli/**/*.ts") to include.

Fixed — added cli/**/*.ts and shared/**/*.ts to tsconfig include.

sf8193 · 2026-07-05T15:04:26Z

-export const SPAWN_MODEL = 'claude-opus-4-6[1m]'
+import { DEFAULT_MODEL } from '../shared/constants.js'
+
+// Frozen at import time — daemon restart required to pick up changes.


Nit (flagged by both reviewers): This import is placed mid-file after the BRIDGE_TOOLS array and other exports. ESM hoisting makes it work at runtime, but it breaks the codebase convention of imports-at-top and may trip linters. Move it up with the other imports at the top of the file.

Fixed — moved to top with the other imports.

sf8193 · 2026-07-05T15:04:26Z

+import { DEFAULT_MODEL } from '../shared/constants.js'
+
+// Frozen at import time — daemon restart required to pick up changes.
+export const SPAWN_MODEL = process.env.HYDRA_MODEL || DEFAULT_MODEL


Nit: process.env.HYDRA_MODEL is not trimmed — a value like " " (whitespace only) would be treated as a valid model ID and passed to claude --model. Same applies to cli/lifecycle.ts:69. Consider process.env.HYDRA_MODEL?.trim() || DEFAULT_MODEL.

Fixed — both env reads now use ?.trim() || DEFAULT_MODEL.

Model was hardcoded in two places (bridge-dispatch SPAWN_MODEL for spawns, lifecycle.ts for the byte). This makes it selectable at two levels, with precedence: `--model` (per-spawn) > HYDRA_MODEL (config default) > built-in. - daemon/bridge-dispatch.ts: SPAWN_MODEL = HYDRA_MODEL ?? DEFAULT_MODEL - cli/lifecycle.ts: byte reads HYDRA_MODEL - daemon/session-lifecycle.ts + sessions.ts: SpawnOpts.model overrides per spawn - cli/hydra.ts + daemon/cli-handler.ts: `hydra spawn --model <id>` - .env.example: document HYDRA_MODEL The daemon self-loads the state .env (config.ts), so HYDRA_MODEL reaches spawns without touching buildDaemonEnvs. Do nothing → identical to today. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…ion model passthrough - Import DEFAULT_MODEL in lifecycle.ts instead of duplicating the string - Use || (falsy) instead of ?? (nullish) so empty-string model falls back correctly - Add model field to spawn_session bridge tool so byte can select models for spawns Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Same empty-string bypass that was fixed for opts.model also existed at the env-read level — HYDRA_MODEL="" would resolve to "" instead of DEFAULT_MODEL. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…llback Review round 2 feedback: - Extract DEFAULT_MODEL to shared/constants.ts — eliminates cross-boundary import (CLI was importing from daemon module graph) - Revert session-lifecycle to ?? (nullish) — bad input should fail visibly at the claude CLI, not silently fall back. HYDRA_MODEL env read keeps || since empty-string-in-.env is a config mistake that should fall back. - Normalize model at bridge tool entry: trim + empty→undefined, so LLM callers sending { model: "" } or { model: " " } get clean fallback - Log model overrides from spawn_session for debuggability - CLI help: show full default chain ($HYDRA_MODEL or claude-opus-4-6[1m]) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- Add cli/**/*.ts and shared/**/*.ts to tsconfig.json include — shared/constants.ts and CLI files were not type-checked - Move DEFAULT_MODEL import to top of bridge-dispatch.ts (convention) - Trim HYDRA_MODEL env reads — whitespace-only values fall back to default Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

sf8193 reviewed Jul 4, 2026

View reviewed changes

sf8193 reviewed Jul 5, 2026

View reviewed changes

Dan Cetlin and others added 5 commits July 5, 2026 09:15

fix(model): use falsy check on HYDRA_MODEL env read

d40e683

Same empty-string bypass that was fixed for opts.model also existed at the env-read level — HYDRA_MODEL="" would resolve to "" instead of DEFAULT_MODEL. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

dcetlin force-pushed the feat/model-selection branch from 395c609 to 0015bad Compare July 5, 2026 15:16

dcetlin merged commit 651a282 into main Jul 5, 2026

		@@ -361,14 +361,16 @@ export async function doSpawnSession(topic: string, chatId?: string, messageId?:
		prompt = buildSpawnPrompt(promptParams)
		}

		@@ -0,0 +1 @@
		export const DEFAULT_MODEL = 'claude-opus-4-6[1m]'

Conversation

dcetlin commented Jul 4, 2026

What

Changes

Why no buildDaemonEnvs change

Verification

Uh oh!

sf8193 left a comment

Choose a reason for hiding this comment

PR #81 Review — configurable default + per-spawn model override

Items not on diff lines

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dcetlin commented Jul 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Why no `buildDaemonEnvs` change